feat(dns,cni): cluster-domain derivation (241/242) + topology-aware peer latency (245) - #27
Conversation
…ology-aware peer latency (245) 241/242 — DNS monitor now derives the in-cluster probe target from /etc/resolv.conf 'search' domains (kubernetes.default.svc.<derived>) instead of hardcoding cluster.local. A hardcoded cluster.local produces a fleet-wide false ClusterDNSResolutionFailed on clusters with a CUSTOM cluster domain (the target does not resolve) — the root cause of the original incident. Derivation is the DEFAULT only when clusterDomains is nil; an explicit empty slice (cluster DNS disabled, the current live mitigation) is left untouched, so this change does NOT re-enable the check on the fleet — re-enabling stays a config decision. 242's pod-network intent is satisfied by probing the derived domain through the cluster resolver (node-doctor uses ClusterFirstWithHostNet). 245 — peer latency is now topology-aware. Discovery tags each peer with its node's topology.kubernetes.io/zone label and computes SameZone relative to this node; cross-zone peers use CrossZoneWarningLatency/CrossZoneCriticalLatency when configured, so normal inter-site WAN latency does not raise a false NetworkDegraded. Inert by default: with no zone labels OR no cross-zone thresholds set, behaviour is unchanged. Tests: cluster-domain derivation (standard/custom/fallback/disabled-stays-disabled), resolv.conf parsing, topology threshold selection + config parsing, discovery zone population (labeled cross/same-zone and unlabeled inert path). Chart RBAC already grants nodes list. Tasks: #19560/#19561 (241/242), #19564 (245). NOTE: DNS cluster-DNS re-enable is a GATED deploy decision (re-enables the incident check) — code shipped, roll deferred.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // cluster domain from its `search` line. Returns ("", false) if the file can't be read | ||
| // or no cluster domain can be identified. | ||
| func deriveClusterDomainFromResolver(resolverPath string) (string, bool) { | ||
| file, err := os.Open(resolverPath) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d14472d400
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if c.ClusterDomains == nil { | ||
| c.ClusterDomains = []string{"kubernetes.default.svc.cluster.local"} | ||
| c.ClusterDomains = defaultClusterDomains(c.ResolverPath) |
There was a problem hiding this comment.
For deployments that omit network-dns-check and let ApplyDefaultMonitors add dns-health, this nil-only default path is never reached: MonitorInfo.DefaultConfig in this file still supplies a non-nil clusterDomains slice containing kubernetes.default.svc.cluster.local, and ApplyDefaultMonitors copies it before parseDNSConfig/applyDefaults. On custom-cluster-domain clusters, that auto-enabled default continues to probe cluster.local and emits the same false ClusterDNSResolutionFailed this change is meant to avoid; make the registered default omit clusterDomains or derive there too.
Useful? React with 👍 / 👎.
| // Check for high latency (collect but don't emit individual events). | ||
| // Cross-zone peers use the looser cross-zone warning threshold when configured, | ||
| // so normal inter-site WAN latency does not raise a false NetworkDegraded. | ||
| if peerStatus.AvgLatency > m.warningLatencyFor(result.peer) { |
There was a problem hiding this comment.
When crossZoneCriticalLatency is set without crossZoneWarningLatency (or when criticalLatency is intentionally the effective lower same-zone threshold), this new check only compares average latency to warningLatencyFor(), which never reads CriticalLatency or CrossZoneCriticalLatency. Those configured critical thresholds therefore have no effect, so cross-zone WAN peers can still be marked degraded at the same-zone warning threshold unless operators also set the new warning threshold.
Useful? React with 👍 / 👎.
2 participants
Summary
Remaining feature-1283 tasks, code + tests only — no prod roll of the DNS cluster-DNS re-enable (it's a gated decision; see below).
241/242 — derive cluster domain from resolv.conf
The DNS monitor now derives the in-cluster probe target from
/etc/resolv.confsearchdomains →kubernetes.default.svc.<derived>, instead of hardcodingcluster.local. Hardcodingcluster.localproduces a fleet-wide false ClusterDNSResolutionFailed on custom-cluster-domain clusters — the root cause of the original incident.Safety: derivation is the default only when
clusterDomainsis nil. An explicit empty slice (clusterDomains: [], the current live mitigation) is left untouched, so this does not re-enable the check on the fleet. 242's pod-network intent is met by probing the derived domain through the cluster resolver (node-doctor runsClusterFirstWithHostNet).245 — topology-aware peer latency
Peer discovery tags each peer with its node's
topology.kubernetes.io/zoneand computesSameZone. Cross-zone peers usecrossZoneWarningLatency/crossZoneCriticalLatencywhen set, so normal inter-site WAN latency doesn't raise a falseNetworkDegraded. Inert by default (no zone labels or no cross-zone thresholds → unchanged behaviour). Chart RBAC already grantsnodes: list.Tests
Re-enabling the cluster-DNS check (setting
clusterDomainsnon-empty to use the new derivation) re-introduces the exact check whose false-positive caused the incident. Merge the code freely; the config change to re-enable must be a watched-window decision. 245 and the derivation code are safe/inert and can ship in a normal v1.8.3 roll.Tasks: #19560/#19561 (241/242), #19564 (245)